Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved Forgot Password Procedure #455

Closed
wants to merge 4 commits into from
Closed

Improved Forgot Password Procedure #455

wants to merge 4 commits into from

Conversation

q2apro
Copy link

@q2apro q2apro commented Oct 20, 2016

Preamble: Did not know how I can prevent the other two commits from 2015...

NEW:

  1. Forgot password, enter username or email
  2. Mail goes to user, front end shows: check your email
  3. User clicks link in email, comes to reset page, data (code and email/handle) gets submitted by JS automatically
  4. User gets the 2nd email that contains his new password, frontend shows: "password was sent to you", and the input fields, cursor in password field blinking
  5. User checks his email, copies the password, done.

Not perfect, but better than before!

@q2apro
Copy link
Author

q2apro commented Oct 20, 2016

That is the new code: b53a844

Displays after redirect from reset the message "check your email" plus
login button
@svivian
Copy link
Collaborator

svivian commented Oct 20, 2016

Thanks for the effort. Not sure if I'm missing something, but does this only submit the form with JS? The process is exactly the same besides that? If so, while I see it's a small improvement it doesn't really solve the problem. The user still gets two emails and their password is still sent via email.

As mentioned on the Q2A site the best solution would be when they click the link in the first email, they're shown a form to put in a new password themselves, rather than be sent a second email.


Also just a few notes on the code itself:

  • For some reason this has been mixed up with other PRs/commits, maybe check the Github docs in case there's something you did wrong.
  • The PR has been sent against the master branch, but for new features it should be against the 1.8 branch. See the contributing guide for details.
  • The coding style seems to be messed up. For example, here the curly brace is on the next line. Maybe this was done automatically by your text editor/IDE? If so, see if you can configure it to follow our style (see the contributing file) or turn off automatic formatting.

Hope that helps for future PRs :)

@q2apro
Copy link
Author

q2apro commented Oct 20, 2016

I am very short on time recently and cannot spent extra hours to tidy up the details, esp. Github (I missed to sync beforehand). Sorry for that.

And yes, the process is improved. We tested it already.

You can leave it open until someone else finds it again and reports back.

I started to use brackets on the new line. I am not sure, saw this at Gideon or someone else.

Greetings.

@DanielRuf
Copy link
Contributor

DanielRuf commented Oct 29, 2016

Very bad idea never send any passwords via email.

  1. User gets the 2nd email that contains his new password, frontend shows: "password was sent to you", and the input fields, cursor in password field blinking

@svivian
Copy link
Collaborator

svivian commented Nov 30, 2016

Solved by #457

@svivian svivian closed this Nov 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants